Fix Work auto-create lane launch and naming#303
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughThis PR refactors chat lane-name suggestion from OpenCode runtime to AI integration, introduces optional session-creation callback metadata for foreground/background launch distinction, implements draft background launch UX with dismiss affordance, adds auto-create testing infrastructure, and includes PTY probe timeout configurability and minor safety fixes. ChangesSession creation callback contract and wiring
Lane name suggestion refactoring from OpenCode to AI integration
Draft background launch implementation with new callback options
Comprehensive auto-create draft testing
PTY smoke and LaneCombobox safety fixes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
arul28
changed the title
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Owner
Author
|
@copilot review but do not make fixes |
Contributor
Review-only pass completed (no fixes made). On bd730e9/4661ce0:
Validation note: local targeted test run could not execute because |
Owner
Author
|
@copilot review but do not make fixes |
Copilot stopped work on behalf of
arul28 due to an error
May 14, 2026 19:47
Owner
Author
|
@copilot review but do not make fixes |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/desktop/src/renderer/components/terminals/TerminalsPage.test.tsx (1)
163-164: ⚡ Quick winReuse
AgentChatSessionCreatedOptionsin the WorkViewArea mock prop typing.The inline
{ activate?: boolean; source?: ... }type can drift from the real callback contract and reduce test reliability.Proposed change
-import type { AgentChatSession, LaneSummary } from "../../../shared/types"; +import type { AgentChatSession, LaneSummary } from "../../../shared/types"; +import type { AgentChatSessionCreatedOptions } from "../chat/AgentChatPane"; vi.mock("./WorkViewArea", () => ({ WorkViewArea: (props: { - onOpenChatSession: (session: AgentChatSession, options?: { activate?: boolean; source?: "chat" | "draft-launch" | "handoff" }) => void | Promise<void>; + onOpenChatSession: ( + session: AgentChatSession, + options?: AgentChatSessionCreatedOptions, + ) => void | Promise<void>; }) => (As per coding guidelines, "Keep IPC contracts, preload types, shared types, and renderer usage in sync whenever an interface changes in TypeScript".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/terminals/TerminalsPage.test.tsx` around lines 163 - 164, The test mock for the WorkViewArea prop 'onOpenChatSession' uses an inline type for the options object which can drift; replace the inline type with the shared type AgentChatSessionCreatedOptions so the test signature stays in sync with production. Locate the mock prop typing for onOpenChatSession in TerminalsPage.test.tsx and import and use AgentChatSessionCreatedOptions (alongside AgentChatSession if needed) instead of the inline { activate?: boolean; source?: "chat" | "draft-launch" | "handoff" } definition, updating any related imports or type annotations to match.apps/desktop/src/renderer/components/chat/AgentChatPane.submit.test.tsx (1)
420-422: ⚡ Quick winUse the concrete session-created options type in this helper signature.
options?: anydrops compile-time coverage for the callback contract and can let interface drift slip through tests.Proposed change
-import { AgentChatPane, isMatchingOptimisticUserMessage } from "./AgentChatPane"; +import { + AgentChatPane, + isMatchingOptimisticUserMessage, + type AgentChatSessionCreatedOptions, +} from "./AgentChatPane"; function renderAutoCreateDraftPane(args?: { - onSessionCreated?: (session: AgentChatSession, options?: any) => void | Promise<void>; + onSessionCreated?: ( + session: AgentChatSession, + options?: AgentChatSessionCreatedOptions, + ) => void | Promise<void>; }) {As per coding guidelines, "Keep IPC contracts, preload types, shared types, and renderer usage in sync whenever an interface changes in TypeScript".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/chat/AgentChatPane.submit.test.tsx` around lines 420 - 422, The helper renderAutoCreateDraftPane's onSessionCreated callback currently uses a loose options?: any parameter; replace that any with the concrete session-created options type used by the AgentChatSession API (e.g., the exported type used wherever sessions are created, such as AgentChatSessionCreatedOptions or SessionCreatedOptions) so the test helper's signature matches the real IPC/renderer contract; import that specific type and update the signature in renderAutoCreateDraftPane (and any test callers) to options?: ThatConcreteType to restore compile-time coverage.apps/desktop/src/main/services/chat/agentChatService.ts (1)
6581-6594: 💤 Low valueExtract hardcoded fallback model IDs to module-level constant; note existing redundancy.
The inline fallback model list is repeated five times throughout this file (lines 5774–5778, 6024–6027, 6219–6223, 6582–6589, and 8040–8044). Extracting these four IDs to a module-level constant would improve maintainability. However, note that
DEFAULT_AUTO_TITLE_MODEL_IDis already defined at line 1581 as"anthropic/claude-haiku-4-5", which is hardcoded again as the first fallback entry—this is redundant. The refactor should eliminate this duplication and apply consistently across all five locations.Regarding the claim about
openai/gpt-5.2deprecation: this model is not currently markeddeprecated: trueinmodelRegistry.ts, so a June 2026 retirement date cannot be verified from the codebase.♻️ Revised extraction
+// Ordered fallback candidates tried after the user/config/default title model. +// Listed in preference order; entries are skipped if not present in the +// available (non-deprecated) registry, so stale IDs degrade gracefully. +const TITLE_MODEL_FALLBACK_CANDIDATES = [ + "openai/gpt-5.4-mini", + "openai/gpt-5.2", + "openai/gpt-5.4", +] as const; @@ - const candidateModelIds = [ - requestedModelId, - config.titleModelId, - DEFAULT_AUTO_TITLE_MODEL_ID, - "anthropic/claude-haiku-4-5", - "openai/gpt-5.4-mini", - "openai/gpt-5.2", - "openai/gpt-5.4", - availableModels[0]?.id, - ].reduce<string[]>((acc, candidate) => { + const candidateModelIds = [ + requestedModelId, + config.titleModelId, + DEFAULT_AUTO_TITLE_MODEL_ID, + ...TITLE_MODEL_FALLBACK_CANDIDATES, + availableModels[0]?.id, + ].reduce<string[]>((acc, candidate) => {Note: Apply to all five occurrences for consistency.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/main/services/chat/agentChatService.ts` around lines 6581 - 6594, The inline fallback model list used when building candidateModelIds is duplicated across the file; extract those hardcoded IDs into a single module-level constant (e.g., FALLBACK_MODEL_IDS or FALLBACK_MODEL_CANDIDATES) and reference that constant inside the candidateModelIds construction (function/variable: candidateModelIds) instead of repeating the array—remove the redundant literal "anthropic/claude-haiku-4-5" from the inline list because DEFAULT_AUTO_TITLE_MODEL_ID already equals that value, and ensure all other occurrences that currently inline the same list are updated to use the new constant so the set of fallbacks is maintained consistently (keep the dynamic availableModels[0]?.id inclusion and the filtering logic with availableIds.has).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/renderer/components/chat/AgentChatPane.tsx`:
- Around line 4290-4297: The code creates a session via createSessionForLane and
calls notifySessionCreated before performing the initial send, but if send
rejects the created session/lane is left behind; either move
notifySessionCreated until after send resolves, or if you must optimistically
notify, catch send failures and roll back the created session by
removing/deleting it and reversing the notification. Concretely: keep the
created variable from createSessionForLane, perform send in a try/catch, and on
catch call the existing cleanup API (e.g., deleteSessionForLane or
removeSession/rollbackSession using created.sessionId) and/or call the inverse
notification (e.g., notifySessionDeleted or revert notifySessionCreated) before
restoring the draft UI; apply the same change at the other occurrence around the
send (lines 4325-4328).
- Around line 4023-4028: notifySessionCreated currently calls onSessionCreated
directly so a synchronous throw escapes Promise.resolve’s catch; change the
invocation to run inside Promise.resolve().then(...) (e.g.
Promise.resolve().then(() => options === undefined ? onSessionCreated(session) :
onSessionCreated(session, options))).then(...) or a single Promise chain and
attach .catch(...) so both synchronous and async errors from onSessionCreated
are caught; update the notifySessionCreated function accordingly.
---
Nitpick comments:
In `@apps/desktop/src/main/services/chat/agentChatService.ts`:
- Around line 6581-6594: The inline fallback model list used when building
candidateModelIds is duplicated across the file; extract those hardcoded IDs
into a single module-level constant (e.g., FALLBACK_MODEL_IDS or
FALLBACK_MODEL_CANDIDATES) and reference that constant inside the
candidateModelIds construction (function/variable: candidateModelIds) instead of
repeating the array—remove the redundant literal "anthropic/claude-haiku-4-5"
from the inline list because DEFAULT_AUTO_TITLE_MODEL_ID already equals that
value, and ensure all other occurrences that currently inline the same list are
updated to use the new constant so the set of fallbacks is maintained
consistently (keep the dynamic availableModels[0]?.id inclusion and the
filtering logic with availableIds.has).
In `@apps/desktop/src/renderer/components/chat/AgentChatPane.submit.test.tsx`:
- Around line 420-422: The helper renderAutoCreateDraftPane's onSessionCreated
callback currently uses a loose options?: any parameter; replace that any with
the concrete session-created options type used by the AgentChatSession API
(e.g., the exported type used wherever sessions are created, such as
AgentChatSessionCreatedOptions or SessionCreatedOptions) so the test helper's
signature matches the real IPC/renderer contract; import that specific type and
update the signature in renderAutoCreateDraftPane (and any test callers) to
options?: ThatConcreteType to restore compile-time coverage.
In `@apps/desktop/src/renderer/components/terminals/TerminalsPage.test.tsx`:
- Around line 163-164: The test mock for the WorkViewArea prop
'onOpenChatSession' uses an inline type for the options object which can drift;
replace the inline type with the shared type AgentChatSessionCreatedOptions so
the test signature stays in sync with production. Locate the mock prop typing
for onOpenChatSession in TerminalsPage.test.tsx and import and use
AgentChatSessionCreatedOptions (alongside AgentChatSession if needed) instead of
the inline { activate?: boolean; source?: "chat" | "draft-launch" | "handoff" }
definition, updating any related imports or type annotations to match.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b4094a2b-b5bb-4054-bb84-487c35d99db6
⛔ Files ignored due to path filters (3)
docs/features/chat/README.mdis excluded by!docs/**docs/features/chat/composer-and-ui.mdis excluded by!docs/**docs/features/terminals-and-sessions/README.mdis excluded by!docs/**
📒 Files selected for processing (10)
apps/desktop/src/main/packagedRuntimeSmoke.tsapps/desktop/src/main/services/chat/agentChatService.suggestLaneName.test.tsapps/desktop/src/main/services/chat/agentChatService.tsapps/desktop/src/renderer/components/chat/AgentChatPane.submit.test.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.tsxapps/desktop/src/renderer/components/terminals/LaneCombobox.tsxapps/desktop/src/renderer/components/terminals/TerminalsPage.test.tsxapps/desktop/src/renderer/components/terminals/TerminalsPage.tsxapps/desktop/src/renderer/components/terminals/WorkStartSurface.tsxapps/desktop/src/renderer/components/terminals/WorkViewArea.tsx
Copilot stopped work on behalf of
arul28 due to an error
May 14, 2026 20:14
Owner
Author
|
@copilot review but do not make fixes |
Copilot stopped work on behalf of
arul28 due to an error
May 14, 2026 20:39
Owner
Author
|
@copilot review but do not make fixes |
Owner
Author
|
@copilot review but do not make fixes |
Copilot stopped work on behalf of
arul28 due to an error
May 14, 2026 21:10
Copilot stopped work on behalf of
arul28 due to an error
May 14, 2026 21:16
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
Note: repo-wide typecheck was blocked locally by missing installed node_modules entries for builder-util-runtime and @types/mdast, both already present in package-lock.json.
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Greptile Summary
This PR fixes two related bugs in the Work auto-create lane flow: foreground-launched chats now navigate to
/workinstead of the Lanes tab, and background-launched chats are surfaced optimistically viaupsertOptimisticChatSessionwithout stealing focus, with a new dismissible notice. It also replaces the single-modelrunOpenCodeTextPromptcall for lane naming with a multi-candidate retry loop throughrunSessionIntelligencePrompt, adds a unique timestamp suffix to prompt-derived fallback names, and hardens error cleanup so auto-created lanes and sessions are rolled back when any step fails.AgentChatPane.tsxmovesnotifySessionCreatedto after a successfulsend, always passesactivate: falsefor draft-launch paths (foreground navigation is delegated toopenLaunchedDraftChat), and adds a full catch-path that deletes the orphaned session/lane on failure.agentChatService.tsintroducesGENERIC_PROMPT_LANE_NAMEconstant,uniquePromptFallbackLaneNamefor collision-safe slug+timestamp names, and a sequential retry loop over candidate models before deterministic fallback.packagedRuntimeSmoke.tsmakes the PTY probe timeout configurable viaADE_PACKAGED_PTY_PROBE_TIMEOUT_MSand switches the Windows probe from PowerShell to the lighter-weightcmd.exe.Confidence Score: 5/5
Safe to merge — all three behavioral fixes (foreground routing, background optimistic surface, lane naming retries) are well-scoped, the error-path lane/session rollback addresses the orphaned-lane concern from the previous review, and test coverage spans all new code paths.
The changes are tightly contained: routing fix in openLaunchedDraftChat, activation gating in TerminalsPage, retry loop in agentChatService. The error-cleanup path is now covered by dedicated tests. The only finding is a documentation inaccuracy about the activate flag value for foreground launches — the runtime behavior is correct.
docs/features/chat/composer-and-ui.md contains an inaccurate description of the foreground activate flag value that a suggestion comment addresses.
Important Files Changed
Sequence Diagram
sequenceDiagram participant User participant AgentChatPane participant agentChatService participant lanes as window.ade.lanes participant chat as window.ade.agentChat participant TerminalsPage User->>AgentChatPane: Send / Launch in background AgentChatPane->>agentChatService: "suggestLaneName({prompt, modelId, fallbackName})" Note over agentChatService: Try requestedModel → configuredModel → defaults<br/>Each failure retries next candidate<br/>Fallback: prompt-slug + timestamp suffix agentChatService-->>AgentChatPane: laneName AgentChatPane->>lanes: "create({name: laneName, parentLaneId})" lanes-->>AgentChatPane: createdLane AgentChatPane->>chat: "createSession({laneId: createdLane.id})" chat-->>AgentChatPane: createdSession AgentChatPane->>chat: "send({sessionId, text, ...})" alt send succeeds chat-->>AgentChatPane: ok AgentChatPane->>TerminalsPage: "onSessionCreated(session, {activate: false, source: draft-launch})" TerminalsPage->>TerminalsPage: upsertOptimisticChatSession (no focus steal) alt foreground mode AgentChatPane->>AgentChatPane: "openLaunchedDraftChat → navigate(/work?laneId&sessionId)" else background mode AgentChatPane->>AgentChatPane: setBackgroundLaunchNotice (dismissible) end else send fails (catch) AgentChatPane->>chat: "delete({sessionId})" AgentChatPane->>lanes: "delete({laneId, force:true})" AgentChatPane->>AgentChatPane: restoreDraftLaunchSnapshot + setError endComments Outside Diff (1)
apps/desktop/src/renderer/components/chat/AgentChatPane.tsx, line 4280-4310 (link)createSessionForLaneis called withnotify: true, notifyOptions: { activate: true }, which immediately fireshandleOpenChatSessioninTerminalsPage(callingwork.selectLane,work.focusSession,work.openSessionTabfor the new lane). ThenopenLaunchedDraftChat(launch)is also called formode === "foreground", setting overlapping work view state and navigating again. The double-activation is harmless in the happy path but could cause brief state flicker. Consider passingnotify: falsefor foreground mode whereopenLaunchedDraftChatalready handles navigation and state.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (6): Last reviewed commit: "merge main into lane" | Re-trigger Greptile